feat: install priority plugins first#1997
Conversation
WalkthroughThe changes in the Changes
Sequence DiagramsequenceDiagram
participant RC as rc.local Script
participant PP as Priority Plugins
participant RP as Remaining Plugins
RC->>PP: Define Priority Plugins Array
RC->>PP: Install Priority Plugins First
RC->>RP: Skip Already Installed Priority Plugins
RC->>RP: Install Remaining Plugins
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
etc/rc.d/rc.local (3)
166-173: Consider enhancing error handling and logging for priority plugin installation.The priority-based installation is a good improvement, but consider these enhancements:
- Add error handling for failed installations
- Log when priority plugins are not found
- Consider moving the priority plugins list to a configuration file for easier maintenance
Here's a suggested implementation:
-PRIORITY_PLUGINS=("unraid.patch.plg" "dynamix.unraid.net.plg" "dynamix.unraid.net.staging.plg") +# Load priority plugins from config file, fallback to defaults if not found +PRIORITY_PLUGINS_CONFIG="$CONFIG/plugins/priority_plugins.cfg" +if [[ -f "$PRIORITY_PLUGINS_CONFIG" ]]; then + mapfile -t PRIORITY_PLUGINS < "$PRIORITY_PLUGINS_CONFIG" +else + PRIORITY_PLUGINS=("unraid.patch.plg" "dynamix.unraid.net.plg" "dynamix.unraid.net.staging.plg") +fi # Install priority plugins first for PRIORITY_PLUGIN in "${PRIORITY_PLUGINS[@]}"; do PRIORITY_PLUGIN_PATH="$CONFIG/plugins/$PRIORITY_PLUGIN" if [[ -f "$PRIORITY_PLUGIN_PATH" ]]; then - /usr/local/sbin/plugin install "$PRIORITY_PLUGIN_PATH" | log + if ! /usr/local/sbin/plugin install "$PRIORITY_PLUGIN_PATH" | log; then + log "Error: Failed to install priority plugin: $PRIORITY_PLUGIN" + fi + else + log "Warning: Priority plugin not found: $PRIORITY_PLUGIN" fi done
174-182: Optimize priority plugin check and enhance progress tracking.The current implementation works but can be optimized:
- Replace space-padded string comparison with more efficient array contains check
- Add progress tracking for better monitoring
Here's a suggested implementation:
# Install remaining plugins shopt -s nullglob +total_plugins=0 +installed_plugins=0 +for plugin in "$CONFIG/plugins/"*.plg; do ((total_plugins++)); done +log "Installing remaining plugins ($total_plugins found)" + for PLUGIN in $CONFIG/plugins/*.plg; do PLUGIN_NAME=$(basename "$PLUGIN") # Skip already installed priority plugins - if [[ " ${PRIORITY_PLUGINS[*]} " == *" $PLUGIN_NAME "* ]]; then + if printf '%s\0' "${PRIORITY_PLUGINS[@]}" | grep -Fxqz -- "$PLUGIN_NAME"; then + log "Skipping already installed priority plugin: $PLUGIN_NAME" continue fi - /usr/local/sbin/plugin install "$PLUGIN" | log + if /usr/local/sbin/plugin install "$PLUGIN" | log; then + ((installed_plugins++)) + log "Progress: $installed_plugins/$total_plugins plugins installed" + else + log "Error: Failed to install plugin: $PLUGIN_NAME" + fi done +log "Plugin installation completed: $installed_plugins/$total_plugins successful" shopt -u nullglob
166-182: Consider architectural improvements for plugin management.The priority-based plugin installation is a good architectural improvement. To make it more robust and maintainable, consider these architectural enhancements:
- Create a dedicated plugin management module to handle installation logic
- Implement a configuration system for plugin priorities
- Add a proper logging and monitoring system for installation progress
- Consider implementing a retry mechanism for failed installations
Would you like me to create an issue to track these architectural improvements?
Summary by CodeRabbit
New Features
Improvements